Repository navigation
fix(mesh): hold device commands across a reconnect instead of losing them - #1291
Merged
Merged
Conversation
…ht back On stop the bridge now ends every event stream with a bye frame: reason "restart" with reconnectAfterMs when a /restart or /update armed a successor, reason "shutdown" otherwise. The successor binds the bridge ~10 s after the old process lets go, so the notice asks for 5 s. talon-node reacts to the restart notice: it waits the advertised delay (clamped to 30 s), then retries every second for up to two minutes instead of climbing its 1 s -> 30 s doubling backoff, which used to bring it back up to ~16 s after the successor was listening. A shutdown bye is a plain closed stream. Nodes that predate the frame skip it, like any non-command kind (TestDecodeCommandFrame over the fixture). The events fixture gains a bye sample; the companion tolerates it.
Collaborator
Author
Independent review (second pass): ready once e914cf2 is in (pushed to this branch)Checked: full diff plus No double delivery: a command is only held when no transport took it, and it is removed from Defect found and fixed: e914cf2
Remaining concerns (ranked)
|
…them A command for a device that is between event streams (every device reconnects after a daemon restart, ~1.6x/day) was written to nobody, or to an unclaimed legacy client, and then waited out its whole timeout (up to 305 s for exec). The bridge now reports where sendToDevice put a frame (claimed / fallback / none) and never hands a device that has claimed a stream to the unclaimed fallback. The mesh holds a command no transport took for native.commandHoldMs (default 20 s, capped by the command's timeout), sends it the moment the device's stream reopens, and otherwise fails fast with "has no live connection to the bridge … was not delivered". The bridge-to-mesh wiring moves to frontend/native/mesh-transport.ts so the real transport can be tested end to end.
…s after a restart The "has claimed a stream" set starts empty in every process, so in the gap the hold exists for (just after a daemon restart) a command for a device that is still reconnecting went to whichever unclaimed shared-token client was back first (the desktop app, the CLI): exec command lines and transfer tokens on the wrong stream, the command counted as delivered and never offered to the device when it returned. A device that has authenticated with its own per-device credential binds every stream it opens to its id, so it is addressable from the moment the process starts: sendToDevice now treats it like a device that has claimed a stream (held, "none"), not as a pre-claim build.
The hold was capped by the command's timeout but the timeout was not extended, so a device back with 0.5 s left got the command: it could ring or exec on the device after the caller was told it did not answer, and a retry would run it twice. A held command now stops being deliverable max(3 s, 10% of its timeout) before the deadline (COMMAND_DELIVERY_MARGIN_MS / _SHARE). The hold timer fails it as undelivered at that point, and deviceConnected also refuses one whose delivery time has passed in case the timer has not fired yet. With the default 12 s timeout the hold is now at most 9 s. Tests: the "delivery margin before the deadline" cases in mesh-service.test.ts; four fail without the fix.
dylanneve1
force-pushed
the
feat/mesh-command-hold
branch
from
October 11, 2026 02:48
e914cf2 to
84790df
Compare
dylanneve1
pushed a commit
that referenced
this pull request
Oct 11, 2026
…veFrontend With #1291 (command hold) on top, createNativeFrontend reached 151 lines and tripped the function-size ratchet in the integrated tree. The restart notice's stop options move into bridgeStopOptions(), no change in behaviour.
This was referenced Oct 11, 2026
Collaborator
Author
|
Stack merge order and integrated test results are in #1286 (comment). This PR is 7th of 8. It is now rebased onto #1292 (merge that first) and adds the late-delivery margin fix (84790df). |
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Oct 11, 2026
# Conflicts: # apps/node/bridge.go # apps/node/restart_notice_test.go # src/frontend/native/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of the mesh HA design (
notes/mesh-ha/DESIGN.md§1.5, §4 Phase 2 "Command outbox"; also mesh audit 2026-10-09 #1).Problem
BridgeServer.sendToDevicewrites to whichever streams claim the device right now. When the device is between streams — every device reconnects after a daemon restart (~1.6×/day), and after any network change orsseMaxLifetimeMscycle — the frame went to nobody, and the command then waited out its whole timeout (12 s default, up to 305 s for exec/transfers) for an answer that could not come.Worse: if any unclaimed legacy client was connected at that moment, a reconnecting device's command (exec command line, one-time transfer token, file bodies) was handed to that client instead. The new test
holds a reconnecting device's command instead of handing it to an unclaimed clientshows main doing this.Change
sendToDevicenow returns where the frame went:"claimed" | "fallback" | "none". A device id that has claimed a stream since the server started is never handed to the unclaimed fallback — it is reconnecting, so the answer is"none". Devices that never claimed (pre-claim builds) keep the fallback exactly as before.BridgeServer.onDeviceStream(listener)fires when a stream claiming a device opens (after it is addressable).MeshTransport.commandmay returnfalse= "no live connection to that device".MeshServicethen holds the command forcommandHoldMs(default 20 000, capped by the command's own timeout), offers it again onMeshService.deviceConnected(deviceId)(oldest first), and otherwise fails fast:Pixel 9 has no live connection to the bridge and did not reconnect within 20s, so "ring" was not delivered.Transports returning
void(all existing ones, tests) count as delivered — no behaviour change for them.native.commandHoldMs(0–300 000; 0 = fail at once).native/index.tsintonative/mesh-transport.ts(unchanged locate path), so the real wiring is tested end to end.Offline-by-presence devices are still refused up front by
dispatchCommand, so the hold only applies to devices that look online — i.e. the reconnecting case.Tests (all fail on
origin/main, pass here)mesh-service.test.ts› MeshService command hold across a reconnect (6): delivered after reconnect; fails fast with "not delivered" inside the hold window (main: burns the 5 s timeout, "did not answer"); not resent after failing; other device's reconnect leaves it held; hold capped by command timeout; zero hold.native-command-hold.test.ts(4), real server + SSE +registerMeshTransport+MeshService: command sent before the device's stream opens (fresh daemon) arrives when it opens (main: never arrives); reconnecting device's exec params are not shown to an unclaimed client and arrive on the new stream (main: leaked to the legacy client); fail fast;sendToDevicereturn values.npm run preflightgreen (gitleaks not installed on this host).Not in this PR
bye/restart notice on shutdown (separate PR) — together they make restarts invisible to tool calls.Stacked on #1292 (merge #1292 first)
This branch is now rebased onto #1292 (
feat/mesh-restart-notice), so until #1292 merges the diff here also shows its commit. Conflict resolved insrc/frontend/native/index.ts: this PR's move ofregisterMeshTransportintomesh-transport.tsis kept, along with #1292'sRESTART_RECONNECT_AFTER_MSand itsserver.stop({ restart }).Late-delivery fix (84790df, reviewer concern #2): a held command is no longer delivered with less than max(3 s, 10% of its timeout) left (
COMMAND_DELIVERY_MARGIN_MS/COMMAND_DELIVERY_MARGIN_SHARE). The hold ends at that point and the command fails with the existing "was not delivered" error, so the caller knows it never ran and a retry can't run it twice.deviceConnectedalso refuses a command whose delivery time has passed, in case the hold timer has not fired yet. With the default 12 s timeout the hold is now at most 9 s. Tests: "delivery margin before the deadline" inmesh-service.test.ts(4 of 5 fail without the fix). ThecommandHoldMsconfig doc mentions the margin.Pre-flight GREEN;
go test -race ./...green.🤖 Generated with Claude Code